Add Amr::derive overloads for all levels #4780
Conversation
Src/Amr/AMReX_Amr.H
Outdated
| //! each Multifab in mf | ||
| void derive (const std::string& name, | ||
| Real time, | ||
| Vector<std::unique_ptr<MultiFab>>& mf, |
There was a problem hiding this comment.
I think we should change this to Vector<MultiFab*> const& mf. There are two main advantages. (1) The user might have Vector<MultiFab> mf. It's hard for them to use the unique_ptr interface. (2) The ownership is unclear in the unique_ptr interface. The function takes Vector<std::unique_ptr<MultiFab>> & mf. Is the function going to resize the vector? is the function going to allocate new MultiFabs? The raw pointer interface give the user a clear signal, because the vector is const that prevents the modification of the raw pointers. It's up to the user to own the MultiFabs in the vector.
Note that amrex provides a helper function for converting from Vector<std::unique_ptr<T>> to Vector<T*>. https://amrex-codes.github.io/amrex/doxygen/namespaceamrex.html#a75a0cde85de080ac29b7c4510da71e16 So if you have Vector<std::unique_ptr<MultiFab>> mf, you can just call derive(..., amrex::GetVecOfPtrs(mf));.
There was a problem hiding this comment.
That makes sense. I've made the change to this overload. Are you happy with leaving the other overload returning Vector<std::unique_ptr<MultiFab>>? I guess the reasons you give don't really apply in this case.
There was a problem hiding this comment.
Yes, returning Vector<std::unique_ptr> is fine.
This allows the user to create a vector of MultiFabs (one per level) with the derived quantity computed on each level. The overloads match those in AmrLevel.
mirenradia
force-pushed
the
derive_vector_multifab
branch
from
75a9c51 to
f7ab597
Compare
|
I merged development branch into this so that the CUDA ci can pass. |
2 participants
Summary
This allows the user to create a vector of
MultiFabs (one per level) with the derived quantity computed on each level. The overloads match those inAmrLevel.Additional background
Checklist
The proposed changes: